Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

[WIP] Use Electron event to set urlbar security state #5501

Merged
merged 1 commit into from
Nov 9, 2016

Conversation

diracdeltas
Copy link
Member

@diracdeltas diracdeltas commented Nov 9, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Requires brave/muon#90
Fix #5238

Auditors: @bsclifton @darkdh

Test Plan:

  1. go to http://dev.ruby.sh/bpoc.html and it should not show up as secure

TODO: add automated test for the hackerone issue

@@ -30,9 +30,8 @@ class UrlBarIcon extends ImmutableComponent {
*/
get isInsecure () {
return this.props.isHTTPPage &&
!this.props.isSecure &&
this.props.isSecure === false &&
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] if isSecure is null, we should show neither secure nor insecure

!this.props.active &&
this.props.loading === false &&
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was no longer needed because security state is set to null when the load starts

Requires brave/muon#90
Fix #5238

Auditors: @bsclifton @darkdh

Test Plan:
1. go to http://dev.ruby.sh/bpoc.html and it should not show up as secure

TODO: add automated test for the hackerone issue
@diracdeltas
Copy link
Member Author

@darkdh this seems to cause a problem with https://mixed-script.badssl.com/ - the icon should show up as insecure after scripts are allowed but it still shows up as secure. seems like a timing issue

@darkdh
Copy link
Member

darkdh commented Nov 9, 2016

And this seems broken too
screen shot 2016-11-09 at 08 31 16

@bbondy
Copy link
Member

bbondy commented Nov 9, 2016

@darkdh did you try with the latest electron which is just updated right now? 1.4.24?

@bbondy
Copy link
Member

bbondy commented Nov 9, 2016

merging for preview-1 builds.

@bbondy bbondy merged commit bc97dd2 into master Nov 9, 2016
@darkdh
Copy link
Member

darkdh commented Nov 9, 2016

sorry my bad. It works fine with 1.4.24

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants